Implement fillOrKill & tests

7 files changed, 233 insertions, 46 deletions

diff --git a/src/0x.js.ts b/src/0x.js.ts
index 0f437e039..6d66c9d86 100644
--- a/src/0x.js.ts
+++ b/src/0x.js.ts
@@ -4,7 +4,6 @@ import {bigNumberConfigs} from './bignumber_config';
 import * as ethUtil from 'ethereumjs-util';
 import contract = require('truffle-contract');
 import * as Web3 from 'web3';
-import * as ethABI from 'ethereumjs-abi';
 import findVersions = require('find-versions');
 import compareVersions = require('compare-versions');
 import {Web3Wrapper} from './web3_wrapper';
@@ -16,8 +15,7 @@ import {ExchangeWrapper} from './contract_wrappers/exchange_wrapper';
 import {TokenRegistryWrapper} from './contract_wrappers/token_registry_wrapper';
 import {ecSignatureSchema} from './schemas/ec_signature_schema';
 import {TokenWrapper} from './contract_wrappers/token_wrapper';
-import {SolidityTypes, ECSignature, ZeroExError} from './types';
-import {Order, SignedOrder} from './types';
+import {SolidityTypes, ECSignature, ZeroExError, Order, SignedOrder} from './types';
 import {orderSchema} from './schemas/order_schemas';
 import * as ExchangeArtifacts from './artifacts/Exchange.json';
 
@@ -132,30 +130,13 @@ export class ZeroEx {
      * Computes the orderHash for a given order and returns it as a hex encoded string.
      */
     public async getOrderHashHexAsync(order: Order|SignedOrder): Promise<string> {
-        const exchangeContractAddr = await this.getExchangeAddressAsync();
         assert.doesConformToSchema('order',
                                    SchemaValidator.convertToJSONSchemaCompatibleObject(order as object),
                                    orderSchema);
 
-        const orderParts = [
-            {value: exchangeContractAddr, type: SolidityTypes.address},
-            {value: order.maker, type: SolidityTypes.address},
-            {value: order.taker, type: SolidityTypes.address},
-            {value: order.makerTokenAddress, type: SolidityTypes.address},
-            {value: order.takerTokenAddress, type: SolidityTypes.address},
-            {value: order.feeRecipient, type: SolidityTypes.address},
-            {value: utils.bigNumberToBN(order.makerTokenAmount), type: SolidityTypes.uint256},
-            {value: utils.bigNumberToBN(order.takerTokenAmount), type: SolidityTypes.uint256},
-            {value: utils.bigNumberToBN(order.makerFee), type: SolidityTypes.uint256},
-            {value: utils.bigNumberToBN(order.takerFee), type: SolidityTypes.uint256},
-            {value: utils.bigNumberToBN(order.expirationUnixTimestampSec), type: SolidityTypes.uint256},
-            {value: utils.bigNumberToBN(order.salt), type: SolidityTypes.uint256},
-        ];
-        const types = _.map(orderParts, o => o.type);
-        const values = _.map(orderParts, o => o.value);
-        const hashBuff = ethABI.soliditySHA3(types, values);
-        const hashHex = ethUtil.bufferToHex(hashBuff);
-        return hashHex;
+        const exchangeContractAddr = await this.getExchangeAddressAsync();
+        const orderHash = utils.getOrderHashHex(order, exchangeContractAddr);
+        return orderHash;
     }
     /**
      * Signs an orderHash and returns it's elliptic curve signature
diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index d3a53a9f7..55ff9068e 100644
--- a/src/contract_wrappers/exchange_wrapper.ts
+++ b/src/contract_wrappers/exchange_wrapper.ts
@@ -7,6 +7,7 @@ import {
     ExchangeContract,
     ExchangeContractErrCodes,
     ExchangeContractErrs,
+    Order,
     OrderValues,
     OrderAddresses,
     SignedOrder,
@@ -126,21 +127,9 @@ export class ExchangeWrapper extends ContractWrapper {
         const exchangeInstance = await this.getExchangeContractAsync();
         await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress);
 
-        const orderAddresses: OrderAddresses = [
-            signedOrder.maker,
-            signedOrder.taker,
-            signedOrder.makerTokenAddress,
-            signedOrder.takerTokenAddress,
-            signedOrder.feeRecipient,
-        ];
-        const orderValues: OrderValues = [
-            signedOrder.makerTokenAmount,
-            signedOrder.takerTokenAmount,
-            signedOrder.makerFee,
-            signedOrder.takerFee,
-            signedOrder.expirationUnixTimestampSec,
-            signedOrder.salt,
-        ];
+        const orderAddresses = this.getOrderAddresses(signedOrder);
+        const orderValues = this.getOrderValues(signedOrder);
+
         const gas = await exchangeInstance.fill.estimateGas(
             orderAddresses,
             orderValues,
@@ -169,6 +158,67 @@ export class ExchangeWrapper extends ContractWrapper {
         this.throwErrorLogsAsErrors(response.logs);
     }
     /**
+     * Attempts to fill a specific amount of an order. If the entire amount specified cannot be filled,
+     * the fill order is abandoned.
+     */
+    public async fillOrKillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber,
+                                      shouldCheckTransfer: boolean, takerAddress: string) {
+        assert.doesConformToSchema('signedOrder',
+                            SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object),
+                            signedOrderSchema);
+        assert.isBigNumber('fillTakerAmount', fillTakerAmount);
+        assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer);
+        await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper);
+
+        const exchangeInstance = await this.getExchangeContractAsync();
+        await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress);
+
+        // Check that fillValue available >= fillTakerAmount
+        const orderHashHex = utils.getOrderHashHex(signedOrder, exchangeInstance.address);
+        const unavailableTakerAmount = await this.getUnavailableTakerAmountAsync(orderHashHex);
+        const remainingTakerAmount = signedOrder.takerTokenAmount.minus(unavailableTakerAmount);
+        if (remainingTakerAmount < fillTakerAmount) {
+            throw new Error(ExchangeContractErrs.INSUFFICIENT_REMAINING_FILL_AMOUNT);
+        }
+
+        const orderAddresses = this.getOrderAddresses(signedOrder);
+        const orderValues = this.getOrderValues(signedOrder);
+
+        const gas = await exchangeInstance.fillOrKill.estimateGas(
+            orderAddresses,
+            orderValues,
+            fillTakerAmount,
+            signedOrder.ecSignature.v,
+            signedOrder.ecSignature.r,
+            signedOrder.ecSignature.s,
+            {
+                from: takerAddress,
+            },
+        );
+        try {
+            const response: ContractResponse = await exchangeInstance.fillOrKill(
+                orderAddresses,
+                orderValues,
+                fillTakerAmount,
+                signedOrder.ecSignature.v,
+                signedOrder.ecSignature.r,
+                signedOrder.ecSignature.s,
+                {
+                    from: takerAddress,
+                    gas,
+                },
+            );
+            this.throwErrorLogsAsErrors(response.logs);
+        } catch (err) {
+            // There is a potential race condition where when the cancellation is broadcasted, a sufficient
+            // fillAmount is available, but by the time the transaction gets mined, it no longer is. Instead of
+            // throwing an invalid jump exception, we would rather give the user a more helpful error message.
+            if (_.includes(err, constants.INVALID_JUMP_IDENTIFIER)) {
+                throw new Error(ZeroExError.INSUFFICIENT_REMAINING_FILL_AMOUNT);
+            }
+        }
+    }
+    /**
      * Subscribe to an event type emitted by the Exchange smart contract
      */
     public async subscribeAsync(eventName: ExchangeEvents, subscriptionOpts: SubscriptionOpts,
@@ -232,8 +282,8 @@ export class ExchangeWrapper extends ContractWrapper {
      * Handling the edge-cases that arise when this happens would require making sure that the user has sufficient
      * funds to pay both the fees and the transfer amount. We decided to punt on this for now as the contracts
      * will throw for these edge-cases.
-     * TODO: Throw errors before calling the smart contract for these edge-cases
-     * TODO: in order to minimize the callers gas costs.
+     * TODO: Throw errors before calling the smart contract for these edge-cases in order to minimize
+     * the callers gas costs.
      */
     private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder,
                                                                                fillTakerAmount: BigNumber.BigNumber,
@@ -316,4 +366,25 @@ export class ExchangeWrapper extends ContractWrapper {
         const exchangeInstance = await this.getExchangeContractAsync();
         return exchangeInstance.ZRX.call();
     }
+    private getOrderAddresses(order: Order|SignedOrder) {
+        const orderAddresses: OrderAddresses = [
+            order.maker,
+            order.taker,
+            order.makerTokenAddress,
+            order.takerTokenAddress,
+            order.feeRecipient,
+        ];
+        return orderAddresses;
+    }
+    private getOrderValues(order: Order|SignedOrder) {
+        const orderValues: OrderValues = [
+            order.makerTokenAmount,
+            order.takerTokenAmount,
+            order.makerFee,
+            order.takerFee,
+            order.expirationUnixTimestampSec,
+            order.salt,
+        ];
+        return orderValues;
+    }
 }
diff --git a/src/globals.d.ts b/src/globals.d.ts
index 164fc2386..567ba016d 100644
--- a/src/globals.d.ts
+++ b/src/globals.d.ts
@@ -47,7 +47,9 @@ declare module 'ethereumjs-util' {
 }
 
 // truffle-contract declarations
-declare interface ContractInstance {}
+declare interface ContractInstance {
+    address: string;
+}
 declare interface ContractFactory {
     setProvider: (providerObj: any) => void;
     deployed: () => ContractInstance;
diff --git a/src/types.ts b/src/types.ts
index a02bd0252..49d8365d1 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -44,7 +44,8 @@ export interface ContractEventObj {
 }
 export type CreateContractEvent = (indexFilterValues: IndexFilterValues,
                                    subscriptionOpts: SubscriptionOpts) => ContractEventObj;
-export interface ExchangeContract {
+
+export interface ExchangeContract extends ContractInstance {
     isValidSignature: {
         call: (signerAddressHex: string, dataHex: string, v: number, r: string, s: string,
                txOpts?: TxOpts) => Promise<boolean>;
@@ -68,6 +69,12 @@ export interface ExchangeContract {
         estimateGas: (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber,
                       shouldCheckTransfer: boolean, v: number, r: string, s: string, txOpts: TxOpts) => number;
     };
+    fillOrKill: {
+        (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber,
+         v: number, r: string, s: string, txOpts: TxOpts): ContractResponse;
+        estimateGas: (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber,
+                      v: number, r: string, s: string, txOpts: TxOpts) => number;
+    };
     filled: {
         call: (orderHash: string) => BigNumber.BigNumber;
     };
@@ -76,7 +83,7 @@ export interface ExchangeContract {
     };
 }
 
-export interface TokenContract {
+export interface TokenContract extends ContractInstance {
     balanceOf: {
         call: (address: string) => Promise<BigNumber.BigNumber>;
     };
@@ -89,7 +96,7 @@ export interface TokenContract {
     approve: (proxyAddress: string, amountInBaseUnits: BigNumber.BigNumber, txOpts: TxOpts) => void;
 }
 
-export interface TokenRegistryContract {
+export interface TokenRegistryContract extends ContractInstance {
     getTokenMetaData: {
         call: (address: string) => Promise<TokenMetadata>;
     };
@@ -127,7 +134,7 @@ export const ExchangeContractErrs = strEnum([
     'INSUFFICIENT_MAKER_FEE_BALANCE',
     'INSUFFICIENT_MAKER_FEE_ALLOWANCE',
     'TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER',
-
+    'INSUFFICIENT_REMAINING_FILL_AMOUNT',
 ]);
 export type ExchangeContractErrs = keyof typeof ExchangeContractErrs;
 
diff --git a/src/utils/constants.ts b/src/utils/constants.ts
index 5a5ba0e0a..ebc8586f3 100644
--- a/src/utils/constants.ts
+++ b/src/utils/constants.ts
@@ -1,4 +1,5 @@
 export const constants = {
     NULL_ADDRESS: '0x0000000000000000000000000000000000000000',
     TESTRPC_NETWORK_ID: 50,
+    INVALID_JUMP_IDENTIFIER: 'invalid JUMP at',
 };
diff --git a/src/utils/utils.ts b/src/utils/utils.ts
index 114b46f6c..1d2e2f908 100644
--- a/src/utils/utils.ts
+++ b/src/utils/utils.ts
@@ -1,5 +1,8 @@
 import * as _ from 'lodash';
 import * as BN from 'bn.js';
+import * as ethUtil from 'ethereumjs-util';
+import * as ethABI from 'ethereumjs-abi';
+import {SolidityTypes, Order} from '../types';
 
 export const utils = {
     /**
@@ -22,6 +25,27 @@ export const utils = {
         const isValid = /^0x[0-9A-F]{64}$/i.test(orderHashHex);
         return isValid;
     },
+    getOrderHashHex(order: Order, exchangeContractAddr: string) {
+        const orderParts = [
+            {value: exchangeContractAddr, type: SolidityTypes.address},
+            {value: order.maker, type: SolidityTypes.address},
+            {value: order.taker, type: SolidityTypes.address},
+            {value: order.makerTokenAddress, type: SolidityTypes.address},
+            {value: order.takerTokenAddress, type: SolidityTypes.address},
+            {value: order.feeRecipient, type: SolidityTypes.address},
+            {value: utils.bigNumberToBN(order.makerTokenAmount), type: SolidityTypes.uint256},
+            {value: utils.bigNumberToBN(order.takerTokenAmount), type: SolidityTypes.uint256},
+            {value: utils.bigNumberToBN(order.makerFee), type: SolidityTypes.uint256},
+            {value: utils.bigNumberToBN(order.takerFee), type: SolidityTypes.uint256},
+            {value: utils.bigNumberToBN(order.expirationUnixTimestampSec), type: SolidityTypes.uint256},
+            {value: utils.bigNumberToBN(order.salt), type: SolidityTypes.uint256},
+        ];
+        const types = _.map(orderParts, o => o.type);
+        const values = _.map(orderParts, o => o.value);
+        const hashBuff = ethABI.soliditySHA3(types, values);
+        const hashHex = ethUtil.bufferToHex(hashBuff);
+        return hashHex;
+    },
     spawnSwitchErr(name: string, value: any) {
         return new Error(`Unexpected switch value: ${value} encountered for ${name}`);
     },
diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts
index 370e741b4..9ef20736f 100644
--- a/test/exchange_wrapper_test.ts
+++ b/test/exchange_wrapper_test.ts
@@ -120,6 +120,84 @@ describe('ExchangeWrapper', () => {
             expect(isValid).to.be.true();
         });
     });
+    describe('#fillOrKillOrderAsync', () => {
+        let makerTokenAddress: string;
+        let takerTokenAddress: string;
+        let coinbase: string;
+        let makerAddress: string;
+        let takerAddress: string;
+        let feeRecipient: string;
+        const fillTakerAmount = new BigNumber(5);
+        const shouldCheckTransfer = false;
+        before(async () => {
+            [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses;
+            tokens = await zeroEx.tokenRegistry.getTokensAsync();
+            const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens();
+            makerTokenAddress = makerToken.address;
+            takerTokenAddress = takerToken.address;
+        });
+        describe('failed fillOrKill', () => {
+            it('should throw if remaining fillAmount is less then the desired fillAmount', async () => {
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
+                );
+                const tooLargeFillAmount = new BigNumber(7);
+                const fillAmountDifference = tooLargeFillAmount.minus(fillableAmount);
+                await zeroEx.token.transferAsync(takerTokenAddress, coinbase, takerAddress, fillAmountDifference);
+                await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, tooLargeFillAmount);
+                await zeroEx.token.transferAsync(makerTokenAddress, coinbase, makerAddress, fillAmountDifference);
+                await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, tooLargeFillAmount);
+
+                return expect(zeroEx.exchange.fillOrKillOrderAsync(
+                    signedOrder, tooLargeFillAmount, shouldCheckTransfer, takerAddress,
+                )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_REMAINING_FILL_AMOUNT);
+            });
+        });
+        describe('successful fills', () => {
+            it('should fill a valid order', async () => {
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
+                );
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(fillableAmount);
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(0);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(0);
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(fillableAmount);
+                await zeroEx.exchange.fillOrKillOrderAsync(signedOrder, fillTakerAmount,
+                                                           shouldCheckTransfer, takerAddress);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount));
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(fillTakerAmount);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(fillTakerAmount);
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount));
+            });
+            it('should partially fill a valid order', async () => {
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
+                );
+                const partialFillAmount = new BigNumber(3);
+                await zeroEx.exchange.fillOrderAsync(signedOrder, partialFillAmount,
+                                                     shouldCheckTransfer, takerAddress);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount));
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(partialFillAmount);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(partialFillAmount);
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount));
+            });
+        });
+    });
     describe('#fillOrderAsync', () => {
         let makerTokenAddress: string;
         let takerTokenAddress: string;
@@ -210,7 +288,7 @@ describe('ExchangeWrapper', () => {
                     )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE);
                 });
             });
-            it('should throw when there a rounding error would have occurred', async () => {
+            it('should throw when a rounding error would have occurred', async () => {
                 const makerAmount = new BigNumber(3);
                 const takerAmount = new BigNumber(5);
                 const signedOrder = await fillScenarios.createAsymmetricFillableSignedOrderAsync(
@@ -309,6 +387,29 @@ describe('ExchangeWrapper', () => {
                 expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress))
                     .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount));
             });
+            it('should fill up to remaining amount if desired fillAmount greater than available amount', async () => {
+                const fillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
+                );
+                const tooLargeFillAmount = new BigNumber(7);
+                const fillAmountDifference = tooLargeFillAmount.minus(fillableAmount);
+                await zeroEx.token.transferAsync(takerTokenAddress, coinbase, takerAddress, fillAmountDifference);
+                await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, tooLargeFillAmount);
+                await zeroEx.token.transferAsync(makerTokenAddress, coinbase, makerAddress, fillAmountDifference);
+                await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, tooLargeFillAmount);
+
+                await zeroEx.exchange.fillOrderAsync(signedOrder, tooLargeFillAmount, shouldCheckTransfer,
+                                                     takerAddress);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(fillAmountDifference);
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress))
+                    .to.be.bignumber.equal(fillableAmount);
+                expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(fillableAmount);
+                expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress))
+                    .to.be.bignumber.equal(fillAmountDifference);
+            });
             it('should fill the valid orders with fees', async () => {
                 const fillableAmount = new BigNumber(5);
                 const makerFee = new BigNumber(1);