Add back require statements and tests (will move to another PR)

2 files changed, 50 insertions, 3 deletions

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, {