diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | src/utils/order_validation_utils.ts | 15 | ||||
| -rw-r--r-- | test/order_validation_test.ts | 156 |
3 files changed, 136 insertions, 36 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b3ce29f82..7c6b9ae62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ v0.20.0 - _October 3, 2017_ * Add `zeroEx.token.getLogsAsync` (#178) * Add `zeroEx.exchange.getLogsAsync` (#178) + * Fixed fees validation when one of the tokens transferred is ZRX (#181) v0.19.0 - _September 29, 2017_ * Made order validation optional (#172) diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index 6f7522c41..0450d26a6 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -121,7 +121,11 @@ export class OrderValidationUtils { } if (!isMakerTokenZRX) { - const makerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); + let makerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); + const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; + if (isTakerTokenZRX) { + makerZRXBalance = makerZRXBalance.plus(fillTakerAmount); + } const makerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync( zrxTokenAddress, signedOrder.maker); @@ -141,6 +145,9 @@ export class OrderValidationUtils { signedOrder.takerTokenAddress, senderAddress); const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; + // exchangeRate is the price of one maker token denominated in taker tokens + const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); + const fillMakerAmount = fillTakerAmount.div(exchangeRate); const requiredTakerAmount = isTakerTokenZRX ? fillTakerAmount.plus(signedOrder.takerFee) : fillTakerAmount; if (requiredTakerAmount.greaterThan(takerBalance)) { @@ -151,7 +158,11 @@ export class OrderValidationUtils { } if (!isTakerTokenZRX) { - const takerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress; + let takerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + if (isMakerTokenZRX) { + takerZRXBalance = takerZRXBalance.plus(fillMakerAmount); + } const takerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, senderAddress); if (signedOrder.takerFee.greaterThan(takerZRXBalance)) { diff --git a/test/order_validation_test.ts b/test/order_validation_test.ts index f625433eb..c1a0a6c8c 100644 --- a/test/order_validation_test.ts +++ b/test/order_validation_test.ts @@ -207,7 +207,7 @@ describe('OrderValidation', () => { .to.be.rejectedWith(ExchangeContractErrs.OrderAlreadyCancelledOrFilled); }); }); - describe('#validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync', () => { + describe('#validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync', () => { describe('should throw when not enough balance or allowance to fulfill the order', () => { const balanceToSubtractFromMaker = new BigNumber(3); const balanceToSubtractFromTaker = new BigNumber(3); @@ -218,22 +218,6 @@ describe('OrderValidation', () => { makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); }); - it('should throw when taker balance is less than fill amount', async () => { - await zeroEx.token.transferAsync( - takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, - ); - return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( - signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerBalance); - }); - it('should throw when taker allowance is less than fill amount', async () => { - const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); - await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, - newAllowanceWhichIsLessThanFillAmount); - return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( - signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); - }); it('should throw when maker balance is less than maker fill amount', async () => { await zeroEx.token.transferAsync( makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, @@ -278,23 +262,6 @@ describe('OrderValidation', () => { signedOrder, fillTakerAmount, zrxTokenAddress, )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerFeeAllowance); }); - it('should throw when taker doesn\'t have enough balance to pay fees', async () => { - const balanceToSubtractFromTaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, - ); - return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( - signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeBalance); - }); - it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { - const newAllowanceWhichIsLessThanFees = makerFee.minus(1); - await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, - newAllowanceWhichIsLessThanFees); - return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( - signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeAllowance); - }); }); describe('should throw on insufficient balance or allowance when makerToken is ZRX', () => { @@ -326,6 +293,95 @@ describe('OrderValidation', () => { )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerAllowance); }); }); + describe('should correctly validate fees amounts if taker token is ZRX', + () => { + let signedOrder: SignedOrder; + let txHash: string; + it('should not throw if maker will have enough ZRX to pay fees after the transfer', async () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, zrxTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + txHash = await zeroEx.token.transferAsync(zrxTokenAddress, makerAddress, coinbase, makerFee); + await zeroEx.awaitTransactionMinedAsync(txHash); + await (orderValidationUtils as any).validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, zrxTokenAddress, + ); + }); + it('should throw if maker will not have enough ZRX to pay fees even after the transfer', async () => { + const makerFee = fillableAmount.plus(1); + const takerFee = fillableAmount.plus(1); + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, zrxTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + txHash = await zeroEx.token.transferAsync(zrxTokenAddress, makerAddress, coinbase, makerFee); + await zeroEx.awaitTransactionMinedAsync(txHash); + return expect( + (orderValidationUtils as any).validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, zrxTokenAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerFeeBalance); + }); + }); + }); + describe('#validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync', () => { + describe('should throw when not enough balance or allowance to fulfill the order', () => { + const balanceToSubtractFromMaker = new BigNumber(3); + const balanceToSubtractFromTaker = new BigNumber(3); + const lackingAllowance = new BigNumber(3); + let signedOrder: SignedOrder; + beforeEach('create fillable signed order', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + }); + it('should throw when taker balance is less than fill amount', async () => { + await zeroEx.token.transferAsync( + takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + ); + return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerBalance); + }); + it('should throw when taker allowance is less than fill amount', async () => { + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); + await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFillAmount); + return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); + }); + }); + describe('should throw when not enough balance or allowance to pay fees', () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + beforeEach('setup', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + }); + it('should throw when taker doesn\'t have enough balance to pay fees', async () => { + const balanceToSubtractFromTaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + ); + return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeBalance); + }); + it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { + const newAllowanceWhichIsLessThanFees = makerFee.minus(1); + await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFees); + return expect((orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeAllowance); + }); + }); describe('should throw on insufficient balance or allowance when takerToken is ZRX', () => { const makerFee = new BigNumber(2); @@ -356,5 +412,37 @@ describe('OrderValidation', () => { )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); }); }); + describe('should correctly validate fees amounts if maker token is ZRX', + () => { + let signedOrder: SignedOrder; + let txHash: string; + it('should not throw if taker will have enough ZRX to pay fees after the transfer', async () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + zrxTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + txHash = await zeroEx.token.transferAsync(zrxTokenAddress, takerAddress, coinbase, takerFee); + await zeroEx.awaitTransactionMinedAsync(txHash); + await (orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, + ); + }); + it('should throw if maker will not have enough ZRX to pay fees even after the transfer', async () => { + const makerFee = fillableAmount.plus(1); + const takerFee = fillableAmount.plus(1); + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + zrxTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + txHash = await zeroEx.token.transferAsync(zrxTokenAddress, takerAddress, coinbase, takerFee); + await zeroEx.awaitTransactionMinedAsync(txHash); + return expect( + (orderValidationUtils as any).validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, takerAddress, zrxTokenAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeBalance); + }); + }); }); }); |