Add checks and tests for expired order and zero fill amount

3 files changed, 26 insertions, 2 deletions

diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index 982a8c0c1..b5d5152dd 100644
--- a/src/contract_wrappers/exchange_wrapper.ts
+++ b/src/contract_wrappers/exchange_wrapper.ts
@@ -274,7 +274,13 @@ export class ExchangeWrapper extends ContractWrapper {
     }
     private async validateCancelOrderAndThrowIfInvalidAsync(order: Order,
                                                             cancelAmount: BigNumber.BigNumber): Promise<void> {
-        // TODO
+        if (cancelAmount.eq(0)) {
+            throw new Error(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO);
+        }
+        const currentUnixTimestampSec = Date.now() / 1000;
+        if (order.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) {
+            throw new Error(ExchangeContractErrs.ORDER_CANCEL_EXPIRED);
+        }
     }
 
     /**
diff --git a/src/types.ts b/src/types.ts
index f2214ba6b..32148df7e 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -121,6 +121,8 @@ export enum ExchangeContractErrCodes {
 
 export const ExchangeContractErrs = strEnum([
     'ORDER_FILL_EXPIRED',
+    'ORDER_CANCEL_EXPIRED',
+    'ORDER_CANCEL_AMOUNT_ZERO',
     'ORDER_REMAINING_FILL_AMOUNT_ZERO',
     'ORDER_FILL_ROUNDING_ERROR',
     'FILL_BALANCE_ALLOWANCE_ERROR',
diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts
index 48d10c1e4..b9400bf04 100644
--- a/test/exchange_wrapper_test.ts
+++ b/test/exchange_wrapper_test.ts
@@ -332,6 +332,7 @@ describe('ExchangeWrapper', () => {
         const fillableAmount = new BigNumber(5);
         let signedOrder: SignedOrder;
         let orderHashHex: string;
+        const cancelAmount = new BigNumber(3);
         before(async () => {
             [coinbase, makerAddress, takerAddress] = userAddresses;
             const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens();
@@ -342,9 +343,24 @@ describe('ExchangeWrapper', () => {
             );
             orderHashHex = await zeroEx.getOrderHashHexAsync(signedOrder);
         });
+        describe('failed cancels', () => {
+            it('should throw when cancel amount is zero', async () => {
+                const zeroCancelAmount = new BigNumber(0);
+                return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, zeroCancelAmount))
+                    .to.be.rejectedWith(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO);
+            });
+            it('should throw when order is expired', async () => {
+                const expirationInPast = new BigNumber(42);
+                const expiredSignedOrder = await fillScenarios.createFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast,
+                );
+                orderHashHex = await zeroEx.getOrderHashHexAsync(expiredSignedOrder);
+                return expect(zeroEx.exchange.cancelOrderAsync(expiredSignedOrder, cancelAmount))
+                    .to.be.rejectedWith(ExchangeContractErrs.ORDER_CANCEL_EXPIRED);
+            });
+        });
         describe('successful cancels', () => {
             it('should cancel an order', async () => {
-                const cancelAmount = new BigNumber(5);
                 await zeroEx.exchange.cancelOrderAsync(signedOrder, cancelAmount);
                 const cancelledAmount = await zeroEx.exchange.getCanceledTakerAmountAsync(orderHashHex);
                 expect(cancelledAmount).to.be.bignumber.equal(cancelAmount);