Merge branch 'fillOrderAsync' into unavailableFilledCancelled

5 files changed, 82 insertions, 44 deletions

diff --git a/src/0x.js.ts b/src/0x.js.ts
index c799e63e9..2bced325f 100644
--- a/src/0x.js.ts
+++ b/src/0x.js.ts
@@ -120,7 +120,7 @@ export class ZeroEx {
     /**
      * Sets default account for sending transactions.
      */
-    public setDefaultAccount(account: string): void {
+    public setTransactionSenderAccount(account: string): void {
         this.web3Wrapper.setDefaultAccount(account);
     }
     /**
diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index 6f6380bb8..8b20d2b4d 100644
--- a/src/contract_wrappers/exchange_wrapper.ts
+++ b/src/contract_wrappers/exchange_wrapper.ts
@@ -107,10 +107,10 @@ export class ExchangeWrapper extends ContractWrapper {
         assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer);
 
         const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync();
-        const exchangeInstance = await this.getExchangeContractAsync();
-
         await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress);
 
+        const exchangeInstance = await this.getExchangeContractAsync();
+
         const orderAddresses: OrderAddresses = [
             signedOrder.maker,
             signedOrder.taker,
@@ -164,7 +164,8 @@ export class ExchangeWrapper extends ContractWrapper {
         if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) {
             throw new Error(FillOrderValidationErrs.EXPIRED);
         }
-        const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker);
+        const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress,
+                                                                        signedOrder.maker);
         const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress);
         const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress,
                                                                               signedOrder.maker);
diff --git a/src/types.ts b/src/types.ts
index 2686c140a..29f7e0ee4 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -147,3 +147,7 @@ export interface TxOpts {
     from: string;
     gas?: number;
 }
+
+export interface TokenAddressBySymbol {
+    [symbol: string]: string;
+}
diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts
index 2746c9e46..77c78470b 100644
--- a/test/exchange_wrapper_test.ts
+++ b/test/exchange_wrapper_test.ts
@@ -11,6 +11,7 @@ import {ZeroEx} from '../src/0x.js';
 import {BlockchainLifecycle} from './utils/blockchain_lifecycle';
 import {orderFactory} from './utils/order_factory';
 import {FillOrderValidationErrs, Token} from '../src/types';
+import {FillScenarios} from './utils/fill_scenarios';
 
 chai.use(dirtyChai);
 chai.use(ChaiBigNumber());
@@ -104,79 +105,82 @@ describe('ExchangeWrapper', () => {
     });
     describe('#fillOrderAsync', () => {
         let tokens: Token[];
+        let makerTokenAddress: string;
+        let takerTokenAddress: string;
+        let fillScenarios: FillScenarios;
+        let takerAddress: string;
         const fillTakerAmountInBaseUnits = new BigNumber(5);
-        let maker: string;
-        let taker: string;
         const addressBySymbol: {[symbol: string]: string} = {};
         const shouldCheckTransfer = false;
         before('fetch tokens', async () => {
+            takerAddress = userAddresses[1];
             tokens = await zeroEx.tokenRegistry.getTokensAsync();
             _.forEach(tokens, token => {
                 addressBySymbol[token.symbol] = token.address;
             });
-        });
-        beforeEach('setup', async () => {
-            maker = userAddresses[0];
-            taker = userAddresses[1];
-            await zeroEx.token.setProxyAllowanceAsync(addressBySymbol.MLN, maker, new BigNumber(5));
-            await zeroEx.token.transferAsync(addressBySymbol.GNT, maker, taker, new BigNumber(5));
-            await zeroEx.token.setProxyAllowanceAsync(addressBySymbol.GNT, taker, new BigNumber(5));
+            const [makerToken, takerToken] = tokens;
+            makerTokenAddress = makerToken.address;
+            takerTokenAddress = takerToken.address;
+            fillScenarios = new FillScenarios(zeroEx, userAddresses, tokens);
         });
         afterEach('reset default account', () => {
-            zeroEx.setDefaultAccount(userAddresses[0]);
+            zeroEx.setTransactionSenderAccount(userAddresses[0]);
         });
         describe('failed fills', () => {
             it('should throw when the fill amount is zero', async () => {
-                const makerAmount = 5;
-                const takerAmount = 5;
-                const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, maker, taker,
-                    makerAmount, addressBySymbol.MLN, takerAmount, addressBySymbol.GNT);
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount,
+                );
                 const zeroFillAmount = new BigNumber(0);
-                zeroEx.setDefaultAccount(taker);
+                zeroEx.setTransactionSenderAccount(takerAddress);
                 expect(zeroEx.exchange.fillOrderAsync(signedOrder, zeroFillAmount, shouldCheckTransfer))
                     .to.be.rejectedWith(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO);
             });
             it('should throw when sender is not a taker', async () => {
-                const makerAmount = 5;
-                const takerAmount = 5;
-                const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, maker, taker,
-                    makerAmount, addressBySymbol.MLN, takerAmount, addressBySymbol.GNT);
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount,
+                );
                 expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer))
                     .to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER);
             });
             it('should throw when order is expired', async () => {
-                const timestampInThePast = new BigNumber(42);
-                const makerAmount = 5;
-                const takerAmount = 5;
-                const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, maker, taker,
-                    makerAmount, addressBySymbol.MLN, takerAmount, addressBySymbol.GNT, timestampInThePast);
-                zeroEx.setDefaultAccount(taker);
+                const expirationInPast = new BigNumber(42);
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount, expirationInPast,
+                );
+                zeroEx.setTransactionSenderAccount(takerAddress);
                 expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer))
                     .to.be.rejectedWith(FillOrderValidationErrs.EXPIRED);
             });
-            it('should throw when not enough balance', async () => {
-                const makerAmount = 10;
-                const takerAmount = 10;
-                const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, maker, taker,
-                    makerAmount, addressBySymbol.MLN, takerAmount, addressBySymbol.GNT);
-                zeroEx.setDefaultAccount(taker);
+            it('should throw when taker balance is less than fill amount', async () => {
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount,
+                );
+                zeroEx.setTransactionSenderAccount(takerAddress);
                 const moreThanTheBalance = new BigNumber(6);
-                const checkTransfer = true;
-                expect(zeroEx.exchange.fillOrderAsync(signedOrder, moreThanTheBalance, checkTransfer))
+                expect(zeroEx.exchange.fillOrderAsync(signedOrder, moreThanTheBalance, shouldCheckTransfer))
                     .to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE);
             });
         });
         describe('successful fills', () => {
             it('should fill the valid order', async () => {
-                const makerAmount = 5;
-                const takerAmount = 5;
-                const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, maker, taker,
-                    makerAmount, addressBySymbol.MLN, takerAmount, addressBySymbol.GNT);
-                zeroEx.setDefaultAccount(taker);
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount,
+                );
+                zeroEx.setTransactionSenderAccount(takerAddress);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(0);
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(fillTakerAmountInBaseUnits);
                 await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer);
-                expect(await zeroEx.token.getBalanceAsync(addressBySymbol.MLN, taker))
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress))
                     .to.be.bignumber.equal(fillTakerAmountInBaseUnits);
-                expect(await zeroEx.token.getBalanceAsync(addressBySymbol.GNT, taker))
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress))
                     .to.be.bignumber.equal(0);
             });
         });
diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts
new file mode 100644
index 000000000..ac233ad50
--- /dev/null
+++ b/test/utils/fill_scenarios.ts
@@ -0,0 +1,29 @@
+import * as BigNumber from 'bignumber.js';
+import {ZeroEx} from '../../src/0x.js';
+import {Token, SignedOrder} from '../../src/types';
+import {orderFactory} from '../utils/order_factory';
+
+export class FillScenarios {
+    private zeroEx: ZeroEx;
+    private userAddresses: string[];
+    private tokens: Token[];
+    constructor(zeroEx: ZeroEx, userAddresses: string[], tokens: Token[]) {
+        this.zeroEx = zeroEx;
+        this.userAddresses = userAddresses;
+        this.tokens = tokens;
+    }
+    public async createAFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string,
+                                                 takerAddress: string, fillableAmount: BigNumber.BigNumber,
+                                                 expirationUnixTimestampSec?: BigNumber.BigNumber):
+                                           Promise<SignedOrder> {
+        const [makerAddress] = this.userAddresses;
+        await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, fillableAmount);
+        await this.zeroEx.token.transferAsync(takerTokenAddress, makerAddress, takerAddress, fillableAmount);
+        await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, fillableAmount);
+
+        const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress,
+            takerAddress, fillableAmount, makerTokenAddress, fillableAmount, takerTokenAddress,
+            expirationUnixTimestampSec);
+        return signedOrder;
+    }
+}