From 84d836f22b4bc5410c5d96137eff261b698b622e Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 9 Apr 2018 10:44:39 -0700 Subject: Add back require statements and tests (will move to another PR) --- .../protocol/Exchange/MixinExchangeCore.sol | 11 ++++-- packages/contracts/test/exchange/core.ts | 42 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index e93b7a3eb..000977cf6 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -109,6 +109,8 @@ contract MixinExchangeCore is // Validate order and maker only if first time seen // TODO: Read filled and cancelled only once if (filled[orderHash] == 0) { + require(order.makerTokenAmount > 0); + require(order.takerTokenAmount > 0); require(isValidSignature(orderHash, order.makerAddress, signature)); } @@ -116,6 +118,7 @@ contract MixinExchangeCore is if (order.takerAddress != address(0)) { require(order.takerAddress == msg.sender); } + require(takerTokenFillAmount > 0); // Validate order expiration if (block.timestamp >= order.expirationTimeSeconds) { @@ -124,14 +127,14 @@ contract MixinExchangeCore is } // Validate order availability - uint256 remainingMakerBuyAmount = safeSub(order.takerTokenAmount, filled[orderHash]); - if (remainingMakerBuyAmount == 0) { + uint256 remainingTakerTokenFillAmount = safeSub(order.takerTokenAmount, filled[orderHash]); + if (remainingTakerTokenFillAmount == 0) { emit ExchangeError(uint8(Errors.ORDER_FULLY_FILLED), orderHash); return fillResults; } // Validate fill order rounding - fillResults.takerTokenFilledAmount = min256(takerTokenFillAmount, remainingMakerBuyAmount); + fillResults.takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenFillAmount); if (isRoundingError(fillResults.takerTokenFilledAmount, order.takerTokenAmount, order.makerTokenAmount)) { emit ExchangeError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash); fillResults.takerTokenFilledAmount = 0; @@ -173,6 +176,8 @@ contract MixinExchangeCore is bytes32 orderHash = getOrderHash(order); // Validate the order + require(order.makerTokenAmount > 0); + require(order.takerTokenAmount > 0); require(order.makerAddress == msg.sender); if (block.timestamp >= order.expirationTimeSeconds) { diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index df4915403..ef3b3b9ee 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -501,6 +501,32 @@ describe('Exchange', () => { return expect(exWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(constants.REVERT); }); + it('should throw if makerTokenAmount is 0', async () => { + signedOrder = orderFactory.newSignedOrder({ + makerTokenAmount: new BigNumber(0), + }); + + return expect(exWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(constants.REVERT); + }); + + it('should throw if takerTokenAmount is 0', async () => { + signedOrder = orderFactory.newSignedOrder({ + takerTokenAmount: new BigNumber(0), + }); + + return expect(exWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(constants.REVERT); + }); + + it('should throw if takerTokenFillAmount is 0', async () => { + signedOrder = orderFactory.newSignedOrder(); + + return expect( + exWrapper.fillOrderAsync(signedOrder, takerAddress, { + takerTokenFillAmount: new BigNumber(0), + }), + ).to.be.rejectedWith(constants.REVERT); + }); + it('should throw if maker balances are too low to fill order', async () => { signedOrder = orderFactory.newSignedOrder({ makerTokenAmount: ZeroEx.toBaseUnitAmount(new BigNumber(100000), 18), @@ -581,6 +607,22 @@ describe('Exchange', () => { return expect(exWrapper.cancelOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(constants.REVERT); }); + it('should throw if makerTokenAmount is 0', async () => { + signedOrder = orderFactory.newSignedOrder({ + makerTokenAmount: new BigNumber(0), + }); + + return expect(exWrapper.cancelOrderAsync(signedOrder, makerAddress)).to.be.rejectedWith(constants.REVERT); + }); + + it('should throw if takerTokenAmount is 0', async () => { + signedOrder = orderFactory.newSignedOrder({ + takerTokenAmount: new BigNumber(0), + }); + + return expect(exWrapper.cancelOrderAsync(signedOrder, makerAddress)).to.be.rejectedWith(constants.REVERT); + }); + it('should be able to cancel a full order', async () => { await exWrapper.cancelOrderAsync(signedOrder, makerAddress); await exWrapper.fillOrderAsync(signedOrder, takerAddress, { -- cgit