Merge branch 'fillOrderAsync' of github.com:0xProject/0x.js into fillOrderAsync

# Conflicts:
#	src/contract_wrappers/exchange_wrapper.ts

6 files changed, 63 insertions, 28 deletions

diff --git a/package.json b/package.json
index 8cef48c95..8b472f2a9 100644
--- a/package.json
+++ b/package.json
@@ -30,7 +30,7 @@
     "pretest:umd": "run-s clean build:*:dev",
     "substitute_umd_bundle": "npm run remove_src_files_not_used_by_tests; shx mv _bundles/* lib/src",
     "remove_src_files_not_used_by_tests": "find ./lib/src \\( -path ./lib/src/utils -o -path ./lib/src/schemas -o -path \"./lib/src/types.*\" \\) -prune -o -type f -print | xargs rm",
-    "run_mocha": "mocha lib/test/**/*_test.js"
+    "run_mocha": "mocha lib/test/**/*_test.js --timeout 3000"
   },
   "config": {
     "artifacts": "Proxy Exchange TokenRegistry Token Mintable EtherToken",
diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index 1311730a0..bf3f10e28 100644
--- a/src/contract_wrappers/exchange_wrapper.ts
+++ b/src/contract_wrappers/exchange_wrapper.ts
@@ -4,6 +4,7 @@ import {
     ECSignature,
     ExchangeContract,
     ExchangeContractErrCodes,
+    FillOrderValidationErrs,
     OrderValues,
     OrderAddresses,
     SignedOrder,
@@ -66,6 +67,8 @@ export class ExchangeWrapper extends ContractWrapper {
         const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync();
         const exchangeInstance = await this.getExchangeInstanceOrThrowAsync();
 
+        this.validateFillOrder(signedOrder, fillAmount, senderAddress, shouldCheckTransfer);
+
         const orderAddresses: OrderAddresses = [
             signedOrder.maker,
             signedOrder.taker,
@@ -108,6 +111,18 @@ export class ExchangeWrapper extends ContractWrapper {
         );
         this.throwErrorLogsAsErrors(response.logs);
     }
+    private validateFillOrder(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string,
+                              shouldCheckTransfer: boolean = true) {
+        if (fillAmount.eq(0)) {
+            throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO);
+        }
+        if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) {
+            throw new Error(FillOrderValidationErrs.NOT_A_TAKER);
+        }
+        if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) {
+            throw new Error(FillOrderValidationErrs.EXPIRED);
+        }
+    }
     private async getExchangeInstanceOrThrowAsync(): Promise<ExchangeContract> {
         const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any));
         return contractInstance as ExchangeContract;
diff --git a/src/types.ts b/src/types.ts
index 18de4d27e..992bb4b00 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -31,7 +31,7 @@ export type OrderAddresses = [string, string, string, string, string];
 
 export type OrderValues = [
     BigNumber.BigNumber, BigNumber.BigNumber, BigNumber.BigNumber,
-    BigNumber.BigNumber, BigNumber.BigNumber, BigNumber.BigNumber,
+    BigNumber.BigNumber, BigNumber.BigNumber, BigNumber.BigNumber
 ];
 
 export interface ExchangeContract {
@@ -79,6 +79,13 @@ export enum ExchangeContractErrCodes {
     ERROR_CANCEL_NO_VALUE, // Order has already been fully filled or cancelled
 }
 
+export const FillOrderValidationErrs = strEnum([
+    'FILL_AMOUNT_IS_ZERO',
+    'NOT_A_TAKER',
+    'EXPIRED',
+]);
+export type FillOrderValidationErrs = keyof typeof FillOrderValidationErrs;
+
 export interface ContractResponse {
     logs: ContractEvent[];
 }
diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts
index 32040bd33..a548d8268 100644
--- a/test/0x.js_test.ts
+++ b/test/0x.js_test.ts
@@ -48,6 +48,7 @@ describe('ZeroEx library', () => {
         const expectedOrderHash = '0x103a5e97dab5dbeb8f385636f86a7d1e458a7ccbe1bd194727f0b2f85ab116c7';
         const order: Order = {
             maker: constants.NULL_ADDRESS,
+            taker: constants.NULL_ADDRESS,
             feeRecipient: constants.NULL_ADDRESS,
             makerTokenAddress: constants.NULL_ADDRESS,
             takerTokenAddress: constants.NULL_ADDRESS,
@@ -59,15 +60,8 @@ describe('ZeroEx library', () => {
             expirationUnixTimestampSec: new BigNumber(0),
         };
         const exchangeAddress = constants.NULL_ADDRESS;
-        it('defaults takerAddress to NULL address', () => {
-            const orderHash = ZeroEx.getOrderHashHex(exchangeAddress, order);
-            expect(orderHash).to.be.equal(expectedOrderHash);
-        });
         it('calculates the order hash', () => {
-            const orderWithZeroTaker = _.assign(order, {
-                taker: constants.NULL_ADDRESS,
-            });
-            const orderHash = ZeroEx.getOrderHashHex(exchangeAddress, orderWithZeroTaker);
+            const orderHash = ZeroEx.getOrderHashHex(exchangeAddress, order);
             expect(orderHash).to.be.equal(expectedOrderHash);
         });
     });
diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts
index b2fb1894b..f929df149 100644
--- a/test/exchange_wrapper_test.ts
+++ b/test/exchange_wrapper_test.ts
@@ -7,7 +7,7 @@ import * as _ from 'lodash';
 import {BlockchainLifecycle} from './utils/blockchain_lifecycle';
 import * as BigNumber from 'bignumber.js';
 import {orderFactory} from './utils/order_factory';
-import {Token} from '../src/types';
+import {FillOrderValidationErrs, Token} from '../src/types';
 import * as Web3 from 'web3';
 import * as dirtyChai from 'dirty-chai';
 import ChaiBigNumber = require('chai-bignumber');
@@ -107,6 +107,9 @@ describe('ExchangeWrapper', () => {
         let tokens: Token[];
         const addressBySymbol: {[symbol: string]: string} = {};
         let networkId: number;
+        let maker: string;
+        let taker: string;
+        const fillAmount = new BigNumber(5);
         const setBalance = async (toAddress: string,
                                   amountInBaseUnits: BigNumber.BigNumber|number,
                                   tokenAddress: string) => {
@@ -126,31 +129,43 @@ describe('ExchangeWrapper', () => {
             });
             networkId = await promisify(web3.version.getNetwork)();
         });
+        beforeEach('get ready for fill', async () => {
+            [maker, taker] = userAddresses;
+            zeroEx.setDefaultAccount(taker);
+            await setAllowance(maker, 5, addressBySymbol.MLN);
+            await setBalance(taker, 5, addressBySymbol.GNT);
+            await setAllowance(taker, 5, addressBySymbol.GNT);
+        });
+        afterEach('reset sender', () => {
+            zeroEx.setDefaultAccount(userAddresses[0]);
+        });
         describe('failed fills', () => {
             it('should throw when the fill amount is zero', async () => {
-                const maker = userAddresses[0];
-                const taker = userAddresses[1];
                 const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, networkId, maker, taker,
                     5, addressBySymbol.MLN, 5, addressBySymbol.GNT);
-                const fillAmount = new BigNumber(0);
                 expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount))
-                    .to.be.rejectedWith('This order has already been filled or cancelled');
+                    .to.be.rejectedWith(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO);
+            });
+            it('should throw when sender is not a taker', async () => {
+                const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, networkId, maker, taker,
+                    5, addressBySymbol.MLN, 5, addressBySymbol.GNT);
+                const notTaker = userAddresses[2];
+                zeroEx.setDefaultAccount(notTaker);
+                expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount))
+                    .to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER);
+            });
+            it('should throw when order is expired', async () => {
+                const OLD_TIMESTAMP = new BigNumber(42);
+                const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, networkId, maker, taker,
+                    5, addressBySymbol.MLN, 5, addressBySymbol.GNT, OLD_TIMESTAMP);
+                expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount))
+                    .to.be.rejectedWith(FillOrderValidationErrs.EXPIRED);
             });
         });
         describe('successful fills', () => {
-            afterEach('reset default account', () => {
-               zeroEx.setDefaultAccount(userAddresses[0]);
-            });
             it('should fill the valid order', async () => {
-                const maker = userAddresses[0];
-                const taker = userAddresses[1];
-                await setAllowance(maker, 5, addressBySymbol.MLN);
-                await setBalance(taker, 5, addressBySymbol.GNT);
-                await setAllowance(taker, 5, addressBySymbol.GNT);
                 const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, networkId, maker, taker,
                     5, addressBySymbol.MLN, 5, addressBySymbol.GNT);
-                const fillAmount = new BigNumber(5);
-                zeroEx.setDefaultAccount(taker);
                 await zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount);
                 expect(await zeroEx.token.getBalanceAsync(addressBySymbol.MLN, taker)).to.be.bignumber.equal(5);
                 expect(await zeroEx.token.getBalanceAsync(addressBySymbol.GNT, taker)).to.be.bignumber.equal(0);
diff --git a/test/utils/order_factory.ts b/test/utils/order_factory.ts
index e41e973ee..c6c6ed927 100644
--- a/test/utils/order_factory.ts
+++ b/test/utils/order_factory.ts
@@ -14,10 +14,14 @@ export const orderFactory = {
         makerTokenAmount: BigNumber.BigNumber|number,
         makerTokenAddress: string,
         takerTokenAmount: BigNumber.BigNumber|number,
-        takerTokenAddress: string): Promise<SignedOrder> {
+        takerTokenAddress: string,
+        expirationUnixTimestampSec?: BigNumber.BigNumber): Promise<SignedOrder> {
         // TODO refactor and check
         const exchangeAddress: string = (ExchangeArtifacts as any).networks[networkId].address;
-        const INF_TIMESTAMP = 2524604400;
+        const INF_TIMESTAMP = new BigNumber(2524604400);
+        expirationUnixTimestampSec = _.isUndefined(expirationUnixTimestampSec) ?
+            INF_TIMESTAMP :
+            expirationUnixTimestampSec;
         const order = {
             maker,
             taker,
@@ -29,7 +33,7 @@ export const orderFactory = {
             takerTokenAddress,
             salt: ZeroEx.generatePseudoRandomSalt(),
             feeRecipient: constants.NULL_ADDRESS,
-            expirationUnixTimestampSec: new BigNumber(INF_TIMESTAMP),
+            expirationUnixTimestampSec,
         };
         const orderHash = ZeroEx.getOrderHashHex(exchangeAddress, order);
         const ecSignature = await zeroEx.signOrderHashAsync(orderHash);